Skip to content

Conversation

@highskore
Copy link
Contributor

@highskore highskore commented Nov 12, 2024

solves #158 to make #160 work

@highskore highskore changed the title [WIP] refactor: remove account dependecies [WIP] refactor: remove account dependencies Nov 12, 2024
@highskore highskore marked this pull request as draft November 12, 2024 10:54
@highskore highskore changed the title [WIP] refactor: remove account dependencies refactor: remove account dependencies Nov 14, 2024
@highskore
Copy link
Contributor Author

highskore commented Nov 14, 2024

@kopy-kat a few issues:

  • Registry dependency is ^0.8.24
  • Seems like memory allocation for < 0.8.26 is not the best so SIMULATE=true doens't work on CI (works locally tho)

@highskore highskore marked this pull request as ready for review November 14, 2024 15:47
@kopy-kat
Copy link
Member

Should we remove registry dep too?

@highskore
Copy link
Contributor Author

highskore commented Nov 14, 2024

Should we remove registry dep too?

done but the 7702 compatibility change added to 7579 ref requries ^0.8.24 for tstore too (we can precompile this tho), but the 4337 repo is ^0.8.23 too

maybe lets just keep it >=0.8.24 <0.9.0 ?

@kopy-kat
Copy link
Member

Yeah that’s mid, maybe we remove the 7579 impl too - but yeah maybe we need to stay higher than 0.8.24

@highskore
Copy link
Contributor Author

highskore commented Nov 15, 2024

Yeah that’s mid, maybe we remove the 7579 impl too - but yeah maybe we need to stay higher than 0.8.24

7579 ref is removed, bumped solc to 0.8.24 >=

All in all this pr removes the kernel, safe7579, 7579ref and registry dependecies

@highskore highskore requested a review from kopy-kat November 15, 2024 09:39
@kopy-kat kopy-kat requested a review from zeroknots November 15, 2024 11:53
@kopy-kat
Copy link
Member

I'm kind of having doubts over essentially scrapping dependencies and needing to manually port over changes. These should be rare tho so it might be fine - wdyt @highskore

@highskore
Copy link
Contributor Author

highskore commented Nov 21, 2024

I'm kind of having doubts over essentially scrapping dependencies and needing to manually port over changes. These should be rare tho so it might be fine - wdyt @highskore

I think it's fine since it's audited contracts which should in theory only update rarely and the porting is not that bad (just updating bytecode most of the time)

@highskore
Copy link
Contributor Author

highskore commented Nov 21, 2024

Let's merge #162 first, will probably need to resolve some conflicts @kopy-kat

@highskore
Copy link
Contributor Author

highskore commented Nov 25, 2024

@kopy-kat Found a nested dependency to erc7579 inside module-bases, do we remove the dependency? We would need to duplicate some code, it's stopping modulekit for being able to use solc 0.8.23 because of the tstore inside latest erc7579 ref.

If not - We could bring back the erc7579 dep here or just merge so I can resolve the conflicts on the downgrade-solc branch and close the PRs

Copy link
Member

@kopy-kat kopy-kat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good to me - I think we should release it as a new major version

before merging could you try using this branch in core-modules and seeing if anything breaks?

@highskore highskore merged commit 0688636 into feature/downgrade-solc Nov 28, 2024
6 checks passed
@highskore highskore deleted the refactor/remove-account-dependecies branch December 9, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants